Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX]l10n_it_vat_statement_communication risolve il problema della regola multi company non funzionante #4353

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

matteoopenf
Copy link
Contributor

@matteoopenf matteoopenf commented Sep 5, 2024

risolve #4354

@matteoopenf matteoopenf marked this pull request as ready for review September 5, 2024 11:02
@francesco-ooops francesco-ooops linked an issue Sep 5, 2024 that may be closed by this pull request
1 task
@matteoopenf matteoopenf force-pushed the 16.0-FIX_IVA_multicompany branch 3 times, most recently from 573f374 to 695fd8d Compare September 9, 2024 12:23
@matteoopenf
Copy link
Contributor Author

@tafaRU per caso si può mergiare anche questa?

@francesco-ooops
Copy link
Contributor

@matteoopenf sistema il nome dei commit e della PR per favore

@francesco-ooops
Copy link
Contributor

ps. si possono squashare o ne servono 2?

@matteoopenf matteoopenf changed the title [FIX]l10n_it_vat_statement_communication fix ...dummy [FIX]l10n_it_vat_statement_communication risolve il problema della regola multi company non funzionante Sep 16, 2024
@matteoopenf
Copy link
Contributor Author

@matteoopenf sistema il nome dei commit e della PR per favore

fatto

@francesco-ooops
Copy link
Contributor

@OCA/local-italy-maintainers posso chiedere una review? Grazie!

Copy link
Contributor

@SirAionTech SirAionTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grazie della PR!
Capisco la correzione del typo, ma potresti chiarire come mai questo typo causa dei problemi?

"Migration of l10n_it_vat_statement_communication - unlink"
" previous multi company rule for 'comunicazione.liquidazione' model"
)
old_vat_comm_multi_company_rule_ref.unlink()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non credo che ricreare la regola sia safe: l'utente potrebbe averla modificata (in fondo è per quello che è noupdate), oppure se fosse collegata ad altri record tramite campi relazionali il collegamento sparirebbe.

Normalmente quando una migrazione deve modificare dei record noupdate è sufficiente aggiornare il record, in questo caso però non si sta modificando il record ma il suo ir.model.data, quindi la migrazione dovrebbe recuperare l'ir.model.data e modificarlo.

Copy link
Contributor

@patrickt-oforce patrickt-oforce Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, lo script di migrazione mira a ricreare la regola, che in ambienti migrati resta alla vecchia versione https://github.com/OCA/l10n-italy/blob/12.0/l10n_it_vat_statement_communication/security/security.xml#L9 causando problemi con ambienti multicompany perché il campo usato nella rule potrebbe non corrispondere al valore della proprietà company nell'env che da 14.0 viene usato per molti dei casi; il modificare ir_model_data è solo una correzione ortografica, comunucazione non lo trovo corretto

Oltretutto aggiornare il record della rule modificando il campo domain_force, è lo stesso che farla ricreare; in entrambe i casi se è stata modificata a mano, le modifiche vanno perse; per quanto riguarda il cambiare l'xmlid si può anche tralasciare

"Migration of l10n_it_vat_statement_communication - unlink"
" previous multi company rule for 'comunicazione.liquidazione' model"
)
old_vat_comm_multi_company_rule_ref.unlink()
Copy link
Contributor

@patrickt-oforce patrickt-oforce Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, lo script di migrazione mira a ricreare la regola, che in ambienti migrati resta alla vecchia versione https://github.com/OCA/l10n-italy/blob/12.0/l10n_it_vat_statement_communication/security/security.xml#L9 causando problemi con ambienti multicompany perché il campo usato nella rule potrebbe non corrispondere al valore della proprietà company nell'env che da 14.0 viene usato per molti dei casi; il modificare ir_model_data è solo una correzione ortografica, comunucazione non lo trovo corretto

Oltretutto aggiornare il record della rule modificando il campo domain_force, è lo stesso che farla ricreare; in entrambe i casi se è stata modificata a mano, le modifiche vanno perse; per quanto riguarda il cambiare l'xmlid si può anche tralasciare

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

l10n_it_vat_statement_communication update multi company rule
4 participants